Ruby: generate overlay discard predicates by nickrolfe · Pull Request #19719 · github/codeql

nickrolfe · 2025-06-10T16:48:59Z

Stacked on top of #19684

Copilot

Pull Request Overview

This PR adds support for overlay-based discard predicates in the Ruby Tree-sitter extractor, enabling selective filtering of AST nodes and files when running on overlay databases.

Introduces OverlayAnnotation and PragmaAnnotation in the QL generator to emit overlay/pragma directives.
Extends ql_gen to produce new predicates (isOverlay, getRawFile, discardFile, discardableAstNode, discardAstNode).
Updates the code generator to conditionally include overlay predicates and adds static definitions in the Ruby QLL.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
shared/tree-sitter-extractor/src/generator/ql_gen.rs	Added functions to generate overlay discard predicates.
shared/tree-sitter-extractor/src/generator/ql.rs	Added `TopLevel::Predicate`, `PragmaAnnotation`, `OverlayAnnotation`, and `Expression::Negation`.
shared/tree-sitter-extractor/src/generator/mod.rs	Renamed `add_metadata_relation` to `overlay_support` and wired in overlay predicates.
ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll	Added overlay/pragma annotations and new predicates for Ruby and ERB modules.

Comments suppressed due to low confidence (2)

shared/tree-sitter-extractor/src/generator/ql.rs:271

Add a space or newline after the closing doc comment for readability, e.g.: write!(f, "/** {} */ ", qldoc)?; so that overlay[...] is clearly separated.

write!(f, "/** {} */", qldoc)?;

shared/tree-sitter-extractor/src/generator/mod.rs:63

Consider adding unit or integration tests to verify that overlay_support correctly causes the metadata relation and overlay predicates to be written.

if overlay_support {

ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll

jbj

I have only some cosmetic comments

ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll

nickrolfe · 2025-06-12T15:32:57Z

@aibaars if you assume the generated QL is correct, would you be able to review the generator changes?

jbj

The QL changes LGTM now, assuming you've tested that it works in practice.

ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll

aibaars

Looks good to me. Is there any harm in generating these predicates for other tree-sitter based extractors as well? if not , we might just as well include them by default.

jbj · 2025-06-19T11:12:08Z

I think @kaspersv determined that pragma[nomagic] isn't needed on any of the predicates. It was only needed in an early compiler prototype. Is that right, Kasper?

Is there any harm in generating these predicates for other tree-sitter based extractors as well?

I don't think there is, but it looks like if overlay_support also guards the addition of the databaseMetadata relation to the dbscheme, so I presume all languages will need a dbscheme upgrade if overlay is universally enabled. That seems like a lot to add to this PR.

nickrolfe · 2025-06-19T11:15:50Z

Is there any harm in generating these predicates for other tree-sitter based extractors as well?

I don't think there is, but it looks like if overlay_support also guards the addition of the databaseMetadata relation to the dbscheme, so I presume all languages will need a dbscheme upgrade if overlay is universally enabled. That seems like a lot to add to this PR.

Right. I don't think we normally bother with upgrade scripts for QL-for-QL, but I would prefer not to deal with updating its dbscheme nonetheless, for now.

kaspersv · 2025-06-19T11:18:13Z

I think @kaspersv determined that pragma[nomagic] isn't needed on any of the predicates. It was only needed in an early compiler prototype. Is that right, Kasper?

Yes, that is correct. In theory they should no longer affect the semantics of overlay analysis and a DCA experiment for Java showed no impact of removing them on overlay analysis accuracy (without nomagic annotations vs baseline).

Which reminds me, I should update the Incremental CodeQL docs to remove the nomagic pragmas in examples.

nickrolfe · 2025-06-19T15:22:48Z

I've removed the pragma[nomagic] annotations.

Looks good to me. Is there any harm in generating these predicates for other tree-sitter based extractors as well? if not , we might just as well include them by default.

As discussed in the extractor PR, I can do a quick follow-up PR that adds the databaseMetadata relation to the QL-for-QL dbscheme, and I can make this simplification at the same time.

The base branch was changed.

ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll


+/** Holds if the database is an overlay. */
+overlay[local]
+private predicate isOverlay() { databaseMetadata("isOverlay", "true") }


ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll


+  /** Gets the file containing the given `node`. */
+  overlay[local]
+  private @file getNodeFile(@ruby_ast_node node) {


ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll

+
+  /** Holds if `file` was extracted as part of the overlay database. */
+  overlay[local]
+  private predicate discardFile(@file file) { isOverlay() and file = getNodeFile(_) }


ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll

+
+  /** Holds if `node` is in the `file` and is part of the overlay base database. */
+  overlay[local]
+  private predicate discardableAstNode(@file file, @ruby_ast_node node) {


ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll

+
+  /** Holds if `node` should be discarded, because it is part of the overlay base and is in a file that was also extracted as part of the overlay database. */
+  overlay[discard_entity]
+  private predicate discardAstNode(@ruby_ast_node node) {


ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll


+  /** Gets the file containing the given `node`. */
+  overlay[local]
+  private @file getNodeFile(@erb_ast_node node) {


ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll

+
+  /** Holds if `file` was extracted as part of the overlay database. */
+  overlay[local]
+  private predicate discardFile(@file file) { isOverlay() and file = getNodeFile(_) }


ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll

+
+  /** Holds if `node` is in the `file` and is part of the overlay base database. */
+  overlay[local]
+  private predicate discardableAstNode(@file file, @erb_ast_node node) {


ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll

+
+  /** Holds if `node` should be discarded, because it is part of the overlay base and is in a file that was also extracted as part of the overlay database. */
+  overlay[discard_entity]
+  private predicate discardAstNode(@erb_ast_node node) {


github-actions bot added the Ruby label Jun 10, 2025

nickrolfe force-pushed the nickrolfe/ruby-overlay-extraction branch from fb89f22 to 576f0d2 Compare June 11, 2025 10:27

nickrolfe force-pushed the nickrolfe/ruby-discard-predicates branch from b74138e to 8a33cb4 Compare June 11, 2025 10:29

nickrolfe marked this pull request as ready for review June 11, 2025 10:31

Copilot AI review requested due to automatic review settings June 11, 2025 10:31

nickrolfe requested a review from a team as a code owner June 11, 2025 10:31

Copilot AI reviewed Jun 11, 2025

View reviewed changes

ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll Outdated Show resolved Hide resolved

jbj reviewed Jun 11, 2025

View reviewed changes

ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll Outdated Show resolved Hide resolved

ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll Outdated Show resolved Hide resolved

ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll Outdated Show resolved Hide resolved

nickrolfe force-pushed the nickrolfe/ruby-discard-predicates branch from 8a33cb4 to acf0fe3 Compare June 11, 2025 11:39

nickrolfe force-pushed the nickrolfe/ruby-overlay-extraction branch from 576f0d2 to 4029b89 Compare June 11, 2025 11:45

nickrolfe requested review from a team as code owners June 11, 2025 11:45

nickrolfe force-pushed the nickrolfe/ruby-discard-predicates branch from acf0fe3 to 4875dcb Compare June 11, 2025 11:45

nickrolfe mentioned this pull request Jun 11, 2025

Ruby: enable overlay compilation #19731

Merged

nickrolfe assigned aibaars Jun 12, 2025

jbj previously approved these changes Jun 13, 2025

View reviewed changes

hvitved reviewed Jun 17, 2025

View reviewed changes

ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll Outdated Show resolved Hide resolved

ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll Outdated Show resolved Hide resolved

ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll Outdated Show resolved Hide resolved

nickrolfe force-pushed the nickrolfe/ruby-discard-predicates branch from 4875dcb to 3d78f35 Compare June 17, 2025 08:47

hvitved previously approved these changes Jun 17, 2025

View reviewed changes

aibaars previously approved these changes Jun 19, 2025

View reviewed changes

nickrolfe force-pushed the nickrolfe/ruby-overlay-extraction branch from 4029b89 to fa24ee6 Compare June 19, 2025 14:57

nickrolfe force-pushed the nickrolfe/ruby-discard-predicates branch from 3d78f35 to c9e7be7 Compare June 19, 2025 15:20

nickrolfe added the no-change-note-required This PR does not need a change note label Jun 19, 2025

nickrolfe force-pushed the nickrolfe/ruby-overlay-extraction branch from fa24ee6 to 665df4b Compare June 19, 2025 15:34

nickrolfe force-pushed the nickrolfe/ruby-discard-predicates branch from c9e7be7 to c968a95 Compare June 19, 2025 15:36

Base automatically changed from nickrolfe/ruby-overlay-extraction to main June 25, 2025 10:39

github-advanced-security bot found potential problems Jun 25, 2025

View reviewed changes

Ruby: generate overlay discard predicates

a9ddf00

nickrolfe force-pushed the nickrolfe/ruby-discard-predicates branch from e36a625 to a9ddf00 Compare June 25, 2025 10:48

hvitved approved these changes Jun 25, 2025

View reviewed changes

nickrolfe merged commit 1e68a7e into main Jun 25, 2025
51 checks passed

nickrolfe deleted the nickrolfe/ruby-discard-predicates branch June 25, 2025 11:18

Conversation

nickrolfe commented Jun 10, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

jbj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nickrolfe commented Jun 12, 2025

Uh oh!

jbj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aibaars left a comment

Choose a reason for hiding this comment

Uh oh!

jbj commented Jun 19, 2025

Uh oh!

nickrolfe commented Jun 19, 2025

Uh oh!

kaspersv commented Jun 19, 2025

Uh oh!

nickrolfe commented Jun 19, 2025

Uh oh!

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants